-
Notifications
You must be signed in to change notification settings - Fork 142
feat: Create admin protected endpoint for creating users #981
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for endearing-brigadeiros-63f9d0 canceled.
|
You could replace or remove this test, which is currently skipped, with your new tests: git-proxy/test/testUserCreation.test.js Lines 42 to 50 in 3718943
|
d6f580d
to
271916f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks good! I've had some problems executing the tests, although some issues were present before.
Let me know if I'm missing something! 🙂
Also, it would be very helpful to execute the |
What's the status of this PR? @jescalada @dcoric |
I’m currently OOO but I plan to polish this up by the end of the week. I will ping here once it is ready for another review |
3f7a0be
to
a4d3526
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #981 +/- ##
==========================================
+ Coverage 77.40% 77.64% +0.23%
==========================================
Files 55 55
Lines 2293 2304 +11
Branches 258 258
==========================================
+ Hits 1775 1789 +14
+ Misses 488 485 -3
Partials 30 30 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@JamieSlome @jescalada I've implemented all the requested changes. I ran into some challenges with the tests, but everything should be working correctly now. When you have a moment, could you please review the updates? It should be all set for merging. Let me know if you spot anything else that needs adjustment - otherwise, it's good to go! |
a4d3526
to
d4d9020
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good so far 👍🏼 I spent some time finding fixes for the failing CLI tests. Hopefully committing those directly will fix things.
@@ -483,6 +483,114 @@ describe('test git-proxy-cli', function () { | |||
}); | |||
}); | |||
|
|||
// *** create user *** | |||
|
|||
describe('test git-proxy-cli :: create-user', function () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These functions fix one of the failing CLI tests:
describe('test git-proxy-cli :: create-user', function () { | |
describe('test git-proxy-cli :: create-user', function () { | |
before(async function () { | |
await helper.addRepoToDb(TEST_REPO_CONFIG); | |
await helper.addUserToDb('testuser', 'testpassword', '[email protected]', 'testGitAccount1'); | |
}); | |
after(async function () { | |
await helper.removeRepoFromDb(TEST_REPO_CONFIG.name); | |
await helper.removeUserFromDb('newuser'); | |
await helper.removeUserFromDb('testuser'); | |
}); |
await helper.runCli(`npx -- @finos/git-proxy-cli login --username admin --password admin`); | ||
|
||
const cli = `npx -- @finos/git-proxy-cli create-user --username newuser --email [email protected] --gitAccount newgit`; | ||
const expectedExitCode = 4; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oddly, this fix didn't get applied previously (will fix the test):
const expectedExitCode = 4; | |
const expectedExitCode = 1; | |
const expectedMessages = null; | |
const expectedErrorMessages = ['Missing required argument: password]; |
try { | ||
const cookies = JSON.parse(fs.readFileSync(GIT_PROXY_COOKIE_FILE, 'utf8')); | ||
|
||
const response = await axios.post( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Linter issue here:
351:11 error 'response' is assigned a value but never used no-unused-vars
Introduce a new API endpoint for creating users and a corresponding command in the git-proxy-cli to interact with this endpoint. The new functionality enables administrators to create new user accounts via the CLI.
Changes
createUser
function topackages/git-proxy-cli/index.js
that handles user creation via an API call. It checks for authentication cookies and makes a POST request to the/api/auth/create-user
endpoint.create-user
command to the CLI using yargs inpackages/git-proxy-cli/index.js
. This command takes username, password, email, gitAccount, and an optional admin flag as arguments./api/auth/create-user
insrc/service/routes/auth.js
that handles the user creation request. It checks for admin privileges and validates the request body before creating the user in the database.packages/git-proxy-cli/test/testCli.test.js
, covering scenarios such as server down, no authentication, missing required fields and successful user creation./api/auth/create-user
route totest/testLogin.test.js
to verify authentication, authorization, data validation, and successful user creation.Impact
create-user
command to thegit-proxy-cli
, enabling administrators to create user accounts directly from the command line./api/auth/create-user
endpoint is now available, allowing authenticated administrators to create new user accounts.axios
library inpackages/git-proxy-cli/index.js
.src/service/routes/auth.js
to create user records.Resolves #980